Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(docs): add steps to call SOAP services #2588

Merged
merged 1 commit into from
Mar 19, 2019
Merged

fix(docs): add steps to call SOAP services #2588

merged 1 commit into from
Mar 19, 2019

Conversation

dhmlau
Copy link
Member

@dhmlau dhmlau commented Mar 14, 2019

Fixes #1766

Description

UPDATED 3/14/2019

The Calling other APIs docs page is updated because we no longer need to manually install @loopback/service-proxy. Instead, we can use lb4 service.

The code snippets and instructions are verified with my repo: https://github.com/dhmlau/loopback4-external-apis.


Old description

Since we no longer need to install @loopback/service-proxy manually and can use lb4 service instead, I'm updating this doc: https://loopback.io/doc/en/lb4/Calling-other-APIs-and-web-services.html.

I think my changes can be generalized to be used for calling REST APIs as well, just that the datasource generation will be different. Will need some work on that.

This is currently a draft PR, because I have a question:

  • Do I need to specify the Maps WSDL binding operations to Node.js methods when creating datasource? (see my comment in the actual code changes)

"wsdl": "http://calculator-webservice.mybluemix.net/calculator?wsdl",
"remotingEnabled": true,
// ADD THIS SNIPPET
"operations": {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@strongloop/loopback-maintainers , do I need to add this? I tried without this snippet and it seems to be working fine. I initially thought that by specifying the operations here, there's less code I need to write in the Service class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The operations is for customizing the method names. See https://loopback.io/doc/en/lb3/SOAP-connector.html#operations-property.

Otherwise, the method name is built from serviceName + portName + operationName

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @raymondfeng. The problem I'm having now is that the method name I'm setting here doesn't seem to be used for generating the methods inside the service interface, i.e. no matter what I set under the operations attribute, it's not being referenced in the code. Is it where it's being used?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not automatically update CalculatorService interface but the method names of the service proxy are impacted. We have yet to generate strongly-typed interface from the SOAP WS.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried without this snippet and it seems to be working fine.
It would be good to hint the developer at the command line about these properties and mark them as (optional)

```

For details, you can refer to the SOAP connector's operations property:
https://github.com/strongloop/loopback-connector-soap#operations-property
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new content looks mostly good. However, I am confused about the new structure of this doc page:

  • Calling SOAP web services
    • Add a datasource
    • Add a service
    • Add a Controller
  • Installation
  • Usage
    • Define a data source for the service backend
    • Declare the service interface
    • Declare service proxies for your controller
    • Make service proxies easier to test

IMO, you should rethink the overall structure of this doc page and decide which content will be shared for all services and which content is specific to SOAP/REST/etc. connectors.

docs/site/Calling-other-APIs-and-Web-Services.md Outdated Show resolved Hide resolved
@dhmlau
Copy link
Member Author

dhmlau commented Mar 14, 2019

@bajtos , thanks for the review. I had a question about the operations mapping so think that it might be easier to have a draft PR to show it. You're right, I haven't done the cleanup yet.

@dhmlau dhmlau marked this pull request as ready for review March 14, 2019 19:02
Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love the new structure 👍

Do we want to preserve information about @serviceProxy decorator?

Also the original content was mentioning ServiceMixin that should be applied on the application class, I think it would be good to preserve it. IIUC, your tutorial expects that the application was scaffolded with "services" options enabled. Could you please run it against a new application that was scaffolded without "services" integration, to see if there are any missing steps to document?

@bajtos bajtos added Docs Service Proxy @loopback/service-proxy labels Mar 15, 2019
@dhmlau dhmlau force-pushed the calling-apis branch 2 times, most recently from 621b4f1 to 5979bd6 Compare March 19, 2019 01:01
@dhmlau
Copy link
Member Author

dhmlau commented Mar 19, 2019

@bajtos, I've incorporated your feedback. Also, I've updated the LB3 LTS date as part of this PR.

@dhmlau
Copy link
Member Author

dhmlau commented Mar 19, 2019

@slnode test please
seems to be stuck at CLA check

@bajtos
Copy link
Member

bajtos commented Mar 19, 2019

Also, I've updated the LB3 LTS date as part of this PR.

It usually better to open a new pull request for such an unrelated change.

@bajtos bajtos requested a review from raymondfeng March 19, 2019 08:35
@emonddr emonddr self-requested a review March 19, 2019 14:18
Copy link
Contributor

@emonddr emonddr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great.

@dhmlau dhmlau merged commit 66da58c into master Mar 19, 2019
@dhmlau dhmlau deleted the calling-apis branch March 19, 2019 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Service Proxy @loopback/service-proxy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants